Fix C++ int type mismatch warnings in core/ops files#1070
Conversation
PR tensorflow#476 (dating from 2021) aimed to resolve numerous warnings emitted by the C++ compiler. Most have already been resolved since the time the PR was written, but some still remained. This PR addresses most (possibly all) the rest that can be addressed. (Some cannot be addressed directly because they come from other libraries.) All of these concern the core ops (`tensorflow_quantum/core/ops/`) and all the warnings are essentially in the same category of thing, so while this PR touches many files, it seems appropriate to handle them together: * Changed the data type of variables `max_num_qubits` and `min_num_qubits` from `int` to `uint64_t` to match the underlying data structures and vector iterations where they were being used. * Changed the data types of some iterator and limit variables (`int q_ind`, `int64_t q_ind`, `int nq`, and `int largest_nq`) to `uint64_t` to resolve compiler warnings when evaluated against the newly updated `max_num_qubits` mentioned above. * Changed the data type of `output_dim_size` from `int` to `size_t` where this variable was directly assigned the result of `maps.size()` and similar standard library size methods. * Retained the data types of variables like `max_psum_length` have been left as `int` because they interface with Protobuf sizes, which are signed ints. * In `configure.sh`, removed `-Wno-unused-but-set-variable` from the flags used during builds of `tensorflow_quantum/core/ops/noise/tfq_.*`, since the underlying reason for the warnings has been addressed.
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors various quantum simulation kernels to use 64-bit integers for qubit counts and dimensions, and removes compiler warning suppressions in the configuration script. Feedback highlights several remaining overflow risks, specifically recommending the use of 64-bit literals for bit-shifts and int64_t for loop indices and lambda parameters in parallelized workloads to handle large batch sizes safely.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates multiple quantum operations to use 64-bit integers for qubit counts, loop indices, and parallel work parameters to prevent overflow and support larger simulations. Feedback focuses on ensuring consistency by updating batch index variables to int64_t where they are derived from 64-bit loop indices. The reviewer also noted that tfq_adj_grad_op.cc was missed in the type update and suggested a performance optimization in tfq_calculate_unitary_op.cc to hoist a repeated bit-shift calculation out of a hot loop.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request performs a comprehensive refactoring of integer types across the codebase, replacing 32-bit integers with 64-bit equivalents (int64_t, uint64_t, and size_t) for qubit counts, loop indices, and tensor dimensions. These changes help prevent potential overflows and improve type safety in simulation kernels. Additionally, the PR removes obsolete compiler warning suppressions in configure.sh and corrects minor typos in source comments. There are no review comments to address, and I have no further feedback to provide.
1 participant
PR #476 (dating from 2021) aimed to resolve numerous warnings emitted by the C++ compiler. Most have already been resolved since the time the PR was written, but some still remained. This PR addresses most (possibly all) the rest that can be addressed. (Some cannot be addressed directly because they come from other libraries.)
All of these concern the core ops (
tensorflow_quantum/core/ops/) and all the warnings are essentially in the same category of thing, so while this PR touches many files, it seems appropriate to handle them together:Changed the data type of variables
max_num_qubitsandmin_num_qubitsfrominttouint64_tto match the underlying data structures and vector iterations where they were being used.Changed the data types of some iterator and limit variables (
int q_ind,int64_t q_ind,int nq, andint largest_nq) touint64_tto resolve compiler warnings when evaluated against the updatedmax_num_qubitsmentioned above.Changed the data type of
output_dim_sizefrominttosize_twhere this variable was directly assigned the result ofmaps.size()and similar standard library size methods.Retained the data types of variables like
max_psum_lengthhave been left asintbecause they interface with Protobuf sizes, which are signed ints.In
configure.sh, removed-Wno-unused-but-set-variablefrom the flags used during builds oftensorflow_quantum/core/ops/noise/tfq_.*, since the underlying reason for the warnings has been addressed by this PR.